-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a PinnedMavenInstallAnalyzer #4773
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, I modeled this after PipAnalyzer
, with a lot of copy and paste.
I did follow my own standards for implementation details and debug logging. E.g., I deleted overridden Javadoc identical to parent.
I anticipate and am happy to iterate on the code to bring it up to local standards.
core/src/main/java/org/owasp/dependencycheck/analyzer/PinnedMavenInstallAnalyzer.java
Show resolved
Hide resolved
core/src/main/java/org/owasp/dependencycheck/analyzer/PinnedMavenInstallAnalyzer.java
Outdated
Show resolved
Hide resolved
80a20fc
to
bbe3147
Compare
core/src/main/java/org/owasp/dependencycheck/analyzer/PinnedMavenInstallAnalyzer.java
Outdated
Show resolved
Hide resolved
6c2d5de
to
9a00290
Compare
`install.json` is a new type of Maven lockfile commonly used in Bazel Java projects. Implement virtual dependency scanning for such files, modeled after the existing PipAnalyzer. In addition to the testing added in this PR, it worked on our install.json file: https://github.com/batfish/batfish/blob/6688b5b49ea695e7b566a0b70403396f580b2805/maven_install.json
9a00290
to
0c28bd6
Compare
I'll leave the codestyle review to the various automated code reviews... results should appear soon now that I triggered the PR workflow. Hope to take a second look at the coding over the weekend. @jeremylong any not automatically checked coding standards you'd like to enforce on the project? |
@dhalperi Up to now no issue, I've only glanced over and picked out some items, but going forward: please do not force-push, as that breaks the review cycle. With regular pushes only the changed bits need (re)review, with a force-push the entire change needs to be (re)reviewed as the delta to the already reviewed code is unclear. |
core/src/main/java/org/owasp/dependencycheck/analyzer/PinnedMavenInstallAnalyzer.java
Outdated
Show resolved
Hide resolved
…venInstallAnalyzer.java Co-authored-by: Jeremy Long <jeremy.long@gmail.com>
Thanks for all the great feedback and useful improvements! What else can I do? |
@dhalperi thanks for the PR! |
Fixes Issue
#4772
Description of Change
install.json
is a new type of Maven lockfile commonly usedin Bazel Java projects. Implement virtual dependency scanning
for such files, modeled after the existing PipAnalyzer.
Have test cases been added to cover the new functionality?
Yes, I think so.
However, I was not able to run
mvn -s settings.xml verify
to success locally, so I did not yet add an IT. I also anticipate some changes to these tests in review.In addition to the testing added in this PR, it worked on our
install.json file:
https://github.com/batfish/batfish/blob/6688b5b49ea695e7b566a0b70403396f580b2805/maven_install.json